Replace pdf_path & gs_stack with Vec
#57
Conversation
|
Fixed |
engine/src/dpx_pdfdev.rs
Outdated
| @@ -236,6 +236,16 @@ impl pdf_tmatrix { | |||
| f: 0., | |||
| } | |||
| } | |||
| pub const fn init() -> Self { | |||
There was a problem hiding this comment.
identity or something like that
|
Do you mind leaving this PR pending for a few days... There're some structural changes that i want to make first. And the state management is too much relying on the global variables (Thus the |
burrbull
force-pushed
the
pdfpathvec
branch
from
d01610f to
b0d9501
Compare
dpx/src/dpx_pdfdev.rs
Outdated
| @@ -266,6 +276,12 @@ impl pdf_coord { | |||
| Self { x: 0., y: 0. } | |||
| } | |||
| } | |||
| impl PartialEq for pdf_coord { | |||
There was a problem hiding this comment.
I think this PartialEq is very weird... Do you think make it a method is a better idea?
There was a problem hiding this comment.
More precisely, it’s symmetric but not transitive, so you shouldn’t implement PartialEq this way.
There was a problem hiding this comment.
PartialEq actually doesn't have to be transitive, Eq have to be. But it's still better to not hide these kind of logic under equal operator。
dpx/src/dpx_pdfdraw.rs
Outdated
| strokecolor: pdf_color_graycolor_new(0.).unwrap(), | ||
| fillcolor: pdf_color_graycolor_new(0.).unwrap(), | ||
| linedash: LineDash::default(), | ||
| linecap: 0, |
There was a problem hiding this comment.
line cap and line join can be turned into enum in a future pr.
dpx/src/dpx_pdfdraw.rs
Outdated
| use std::sync::Mutex; | ||
|
|
||
| lazy_static! { | ||
| static ref gs_stack: Mutex<Vec<pdf_gstate>> = Mutex::new({ |
There was a problem hiding this comment.
i'm not very happy with the Mutex here. Can we add a struct called PdfDev or pdf_dev and collect these global states within it. and make the drawing invocations its method? (might be a big change to the code base)
There was a problem hiding this comment.
Helpful for seeing which statics are linked to by C++:
rg 'static mut (\w+):' myfile -o -r '$1' | xargs -n1 -I {} sh -c 'echo {} && rg -F {} c_sources_directory'Generally this helps for moving definitions out of extern “C” when they don’t need to be in one.
There was a problem hiding this comment.
i'm not very happy with the Mutex here. Can we add a struct called PdfDev or pdf_dev and collect these global states within it. and make the drawing invocations its method? (might be a big change to the code base)
I thought about this variant too. The problem as you say, it is a big change. And most likely we will be able to do this at one of the last steps of "oxidation".
There was a problem hiding this comment.
Mutex moved to start of file. Comment added.
There was a problem hiding this comment.
burrbull
force-pushed
the
pdfpathvec
branch
from
b0d9501 to
b7c3a27
Compare
burrbull
force-pushed
the
pdfpathvec
branch
from
b7c3a27 to
a70c783
Compare
|
Let's merge it, i think. |
use
Option<CString>forspot_color_nameinpdf_colorand other changes indpx_pdfdraw.rsNote:issue393_ungetctest doesn't pass now. Panics on emptygs_stack. I think it is not my fault.